Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CRL checking for client certificate #39

Merged
merged 3 commits into from
Oct 19, 2013
Merged

Add CRL checking for client certificate #39

merged 3 commits into from
Oct 19, 2013

Conversation

Vagabond
Copy link
Contributor

If the client provides a certificate, make sure it is actually valid AND
not revoked.

See also basho/riak_test#416

If the client provides a certificate, make sure it is actually valid AND
not revoked.
@Vagabond
Copy link
Contributor Author

Oh, for this to actually work, you'll need to patch public_key:

erlang/otp#97

fetch(Rest);
fetch([{uniformResourceIdentifier, "http"++_=URL}|Rest]) ->
lager:debug("getting CRL from ~p~n", [URL]),
inets:start(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to worry about an error being returned here because it's already been started?

@cmeiklejohn
Copy link
Contributor

I don't mean to be overly pedantic here, but adding some docstrings to the functions would go a long way to being able to revisit this code later and know why it does certain things.

%% XXX the 'Issuer' we get passed here is actually a lie, public key treats the Authority Key Identifier as the 'issuer'
%% Read the CA certs out of the file
%{ok, Bin} = file:read_file(CACerts),
%CACerts = load_certs("/home/andrew/lavabit"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These commented lines probably shouldn't be here.

Add comments; verify clean Dialyzer, test against Riak Test, and
document SSL functions.
@cmeiklejohn
Copy link
Contributor

👍 once #40 is merged. Should address most of my comments in this PR.

Vagabond added a commit that referenced this pull request Oct 19, 2013
Add CRL checking for client certificate
@Vagabond Vagabond merged commit 9885f92 into develop Oct 19, 2013
@seancribbs seancribbs deleted the adt-pb-crl branch April 1, 2015 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants